Skip to content

Delete unused objective-c++ files#631

Merged
msluszniak merged 3 commits intosoftware-mansion:mainfrom
msluszniak:@ms/remove-unused-files
Oct 2, 2025
Merged

Delete unused objective-c++ files#631
msluszniak merged 3 commits intosoftware-mansion:mainfrom
msluszniak:@ms/remove-unused-files

Conversation

@msluszniak
Copy link
Copy Markdown
Member

@msluszniak msluszniak commented Oct 1, 2025

Description

As in the title, utils defined in the ios/RnExecutorch/utils are not longer used so they can be deleted.

Introduces a breaking change?

  • Yes
  • No

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Other (chores, tests, code style improvements etc.)

Tested on

  • iOS
  • Android

Testing instructions

Screenshots

Related issues

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional notes

@msluszniak
Copy link
Copy Markdown
Member Author

msluszniak commented Oct 1, 2025

@chmjkb Can we also delete objective-c++ installers, namely: https://github.com/software-mansion/react-native-executorch/blob/main/packages/react-native-executorch/ios/RnExecutorch/ETInstaller.mm and header for it?

@msluszniak msluszniak requested a review from chmjkb October 1, 2025 18:16
@msluszniak msluszniak self-assigned this Oct 1, 2025
@msluszniak
Copy link
Copy Markdown
Member Author

msluszniak commented Oct 1, 2025

From CI it seems that ScalarType.mm is still used in compilation of LLM demo app. I don't see where it is included though.

@chmjkb
Copy link
Copy Markdown
Collaborator

chmjkb commented Oct 2, 2025

@chmjkb Can we also delete objective-c++ installers, namely: https://github.com/software-mansion/react-native-executorch/blob/main/packages/react-native-executorch/ios/RnExecutorch/ETInstaller.mm and header for it?

This one we're actually using

From CI it seems that ScalarType.mm is still used in compilation of LLM demo app. I don't see where it is included though.

I'll check it out

@NorbertKlockiewicz
Copy link
Copy Markdown
Contributor

NorbertKlockiewicz commented Oct 2, 2025

@msluszniak can you try to remove the cocoapods caching part from build-ios workflow file. I believe it's causing the problems with CI fails.

@NorbertKlockiewicz
Copy link
Copy Markdown
Contributor

NorbertKlockiewicz commented Oct 2, 2025

@msluszniak can you try to remove the cocoapods caching part from build-ios workflow file. I believe it's causing the problems with CI fails.

image

The install cocoapods action still hit the cache as it has if that checks if the Pods directory exists, can you please also remove it and try again?

NorbertKlockiewicz
NorbertKlockiewicz approved these changes Oct 2, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@NorbertKlockiewicz NorbertKlockiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@msluszniak
Copy link
Copy Markdown
Member Author

msluszniak commented Oct 2, 2025

@NorbertKlockiewicz should these cache things from ios workflow be deleted permanently?

@NorbertKlockiewicz
Copy link
Copy Markdown
Contributor

NorbertKlockiewicz commented Oct 2, 2025

@NorbertKlockiewicz should these cache things from ios workflow be deleted permanently?

Yes, we already cache native folders and installing the cocoapods dependencies doesn't take that much time.

@msluszniak msluszniak merged commit 13e1e8e into software-mansion:main Oct 2, 2025
4 checks passed
@msluszniak msluszniak deleted the @ms/remove-unused-files branch October 2, 2025 11:03
KnextKoder pushed a commit to Synkhiv/react-native-executorch that referenced this pull request Nov 7, 2025
@msluszniak
Delete unused objective-c++ files (software-mansion#631)
0c7dcef 
## Description

As in the title, utils defined in the `ios/RnExecutorch/utils` are not
longer used so they can be deleted.

### Introduces a breaking change?

- [ ] Yes
- [X] No

### Type of change

- [ ] Bug fix (change which fixes an issue)
- [ ] New feature (change which adds functionality)
- [ ] Documentation update (improves or adds clarity to existing
documentation)
- [x] Other (chores, tests, code style improvements etc.)

### Tested on

- [ ] iOS
- [ ] Android

### Testing instructions

<!-- Provide step-by-step instructions on how to test your changes.
Include setup details if necessary. -->

### Screenshots

<!-- Add screenshots here, if applicable -->

### Related issues

<!-- Link related issues here using #issue-number -->

### Checklist

- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [ ] My changes generate no new warnings

### Additional notes

<!-- Include any additional information, assumptions, or context that
reviewers might need to understand this PR. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NorbertKlockiewicz NorbertKlockiewicz NorbertKlockiewicz approved these changes

@chmjkb chmjkb Awaiting requested review from chmjkb

Assignees

@msluszniak msluszniak

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@msluszniak @chmjkb @NorbertKlockiewicz